[release][ci] First test for kuberay release test trigger path#54415
[release][ci] First test for kuberay release test trigger path#54415
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extends the release test framework to support a KubeRay variant and updates unit tests accordingly.
- Test changes to initialize a KubeRay compute config and exercise the new
kuberayflag in error scenarios. - Glue code adds exception capture in the KubeRay path, records runtime, and maps exceptions to
Result.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| release/ray_release/tests/test_glue.py | Set up self.kuberay_test, extend _run to take a kuberay flag, and duplicate error-path tests for KubeRay. |
| release/ray_release/glue.py | Wrap KubeRay execution in try/except, record start_time/runtime, call handle_exception, and rethrow. |
Comments suppressed due to low confidence (3)
release/ray_release/tests/test_glue.py:320
- [nitpick] All error paths for KubeRay are covered, but there is no test for a successful KubeRay run. Consider adding a positive test to verify that
self._run(result, True)setsreturn_code == 0and populates expected fields.
self.assertEqual(result.return_code, ExitCode.CONFIG_ERROR.value)
release/ray_release/tests/test_glue.py:259
- [nitpick] The parameter name
kuberayis a bit ambiguous as a flag. Consider renaming to something likeuse_kuberayoris_kuberayfor clarity.
def _run(self, result: Result, kuberay: bool = False, **kwargs):
release/ray_release/tests/test_glue.py:317
- This assertion is indented inside the
with self.assertRaisesRegex(...)block, so it never runs after the exception is raised. Move it outside thewithat the same indentation level to ensure it's executed.
self.assertEqual(result.return_code, ExitCode.CONFIG_ERROR.value)
| working_dir_upload_path = upload_working_dir(get_working_dir(test)) | ||
| start_time = time.monotonic() | ||
| pipeline_exception = None | ||
| try: |
There was a problem hiding this comment.
this wrapping does not really feel right.. to start, catching all Exception is not the right thing to do in most cases. could you explain what exactly you are trying to do?
There was a problem hiding this comment.
I'm using try/catch here mostly to retain the Exception that can be thrown any time commands run inside try. run_release_test_anyscale also implements the same thing. The purpose (I think) is to throw the right error code that matches with whatever errored out during the process (thus why it needs to retain the right exception msg)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com>
…roject#54415) - Modify Kuberay release test trigger code to catch exception and store it in `Result` - Modify glue unit test to include Kuberay variant of the release test trigger path --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: dshepelev15 <d-shepelev@list.ru>
…roject#54415) - Modify Kuberay release test trigger code to catch exception and store it in `Result` - Modify glue unit test to include Kuberay variant of the release test trigger path --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: alimaazamat <alima.azamat2003@gmail.com>
…roject#54415) - Modify Kuberay release test trigger code to catch exception and store it in `Result` - Modify glue unit test to include Kuberay variant of the release test trigger path --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Krishna Kalyan <krishnakalyan3@gmail.com>
…roject#54415) - Modify Kuberay release test trigger code to catch exception and store it in `Result` - Modify glue unit test to include Kuberay variant of the release test trigger path --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>
…roject#54415) - Modify Kuberay release test trigger code to catch exception and store it in `Result` - Modify glue unit test to include Kuberay variant of the release test trigger path --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Kevin H. Luu <kevin@anyscale.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
Result